Skip to content

Conversation

Kamlesh72
Copy link
Contributor

Linked Issue(s)

If team has a repo, then same repo added to another team is not visible.

Also the method getSelectedReposForOrg is returning deleted teams repo, unnecessarily increasing response size.

Acceptance Criteria fulfillment

  • Same repo now visible in all teams it is added to
  • Mark teamRepos inactive if team is deleted
  • Removed unnecessary code

Proposed changes (including videos or screenshots)

The db query now directly returns Team repos with multiple workflows.

Copy link
Member

@jayantbh jayantbh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I only did a shallow review. The team should do a proper review.

@shivam-bit
Copy link
Contributor

@Kamlesh72 Please let me know, once the query changes are done and if possible upload a screen recording for the same.

@Kamlesh72
Copy link
Contributor Author

Kamlesh72 commented Oct 10, 2024

@shivam-bit This works fine.

.leftJoin({ rw: Table.RepoWorkflow }, function () {
  this.on('OrgRepo.id', 'rw.org_repo_id').andOn(
    'rw.is_active',
    '=',
    dbRaw.raw(true)
  );
})

Both this code leaves few repos if their RepoWorkflow exists but not active.

.leftJoin({ rw: Table.RepoWorkflow }, 'OrgRepo.id', 'rw.org_repo_id')
  .andWhere('rw.is_active', true)
  .orWhereNull('rw.is_active');
  
.leftJoin({ rw: Table.RepoWorkflow }, 'OrgRepo.id', 'rw.org_repo_id')
  .andWhere(function () {
    this.where('rw.is_active', true).orWhereNull('rw.is_active');
  })

@shivam-bit
Copy link
Contributor

Both this code leaves few repos if their RepoWorkflow exists but not active.

.leftJoin({ rw: Table.RepoWorkflow }, 'OrgRepo.id', 'rw.org_repo_id')
  .andWhere('rw.is_active', true)
  .orWhereNull('rw.is_active');
  
.leftJoin({ rw: Table.RepoWorkflow }, 'OrgRepo.id', 'rw.org_repo_id')
  .andWhere(function () {
    this.where('rw.is_active', true).orWhereNull('rw.is_active');
  })

@Kamlesh72 if i am getting this right what you mean is that in the returned repos list there will be few repos missing since they don't have any active workflows. If yes, then that shouldn't be the case

@Kamlesh72
Copy link
Contributor Author

@shivam-bit Yes.

It only returns If there is no workflow in db or is_active is true.

I think this will solve it. Will push the code after testing.

.leftJoin(
  { rw: Table.RepoWorkflow },
  function () {
    this.on('OrgRepo.id', 'rw.org_repo_id').andOn(
      'rw.is_active',
      'tr.is_active'
    );
  }
);

@Kamlesh72
Copy link
Contributor Author

@shivam-bit Updated the code.

Previous V/s Current:

Screenshot 2024-10-10 at 8 35 14 PM Screenshot 2024-10-10 at 8 28 31 PM

@Kamlesh72
Copy link
Contributor Author

Will update the Delete team code in morning.

@Kamlesh72
Copy link
Contributor Author

@shivam-bit Does the delete team queries need to be wrapped in transaction?

@shivam-bit
Copy link
Contributor

shivam-bit commented Oct 12, 2024

@shivam-bit Does the delete team queries need to be wrapped in transaction?

@Kamlesh72 Although we are not using using transaction BFF(next.js api) side yet, but in this case it totally make sense to use it.
So feel free to add one.

@shivam-bit
Copy link
Contributor

@Kamlesh72 Lets goooooooo!!🚀

@shivam-bit shivam-bit requested a review from jayantbh October 15, 2024 05:01
@jayantbh jayantbh merged commit f582c4d into middlewarehq:main Oct 15, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants